Skip to content

Comments

Support Non-Nimble icons in the Nimble table#2813

Closed
hellovolcano wants to merge 8 commits intomainfrom
users/vgleason/non-nimble-icons-hld
Closed

Support Non-Nimble icons in the Nimble table#2813
hellovolcano wants to merge 8 commits intomainfrom
users/vgleason/non-nimble-icons-hld

Conversation

@hellovolcano
Copy link
Contributor

Pull Request

🤨 Rationale

Clients need a way to display non-nimble icons, specifically SVG and PNG, in a Nimble table. This HLD proposes adding additional mappings for an SVG and PNG so they can be displayed.

👩‍💻 Implementation

N/A

🧪 Testing

N/A

✅ Checklist

  • I have updated the project documentation to reflect my changes or determined no changes are needed.

@hellovolcano hellovolcano changed the title Users/vgleason/non nimble icons hld Support Non-Nimble icons in the Nimble table Jan 8, 2026
@hellovolcano hellovolcano changed the title Support Non-Nimble icons in the Nimble table [Draft] Support Non-Nimble icons in the Nimble table Jan 8, 2026
@hellovolcano hellovolcano changed the title [Draft] Support Non-Nimble icons in the Nimble table Support Non-Nimble icons in the Nimble table Jan 22, 2026
@hellovolcano hellovolcano marked this pull request as ready for review January 22, 2026 20:42
rajsite added a commit that referenced this pull request Jan 23, 2026
# Pull Request

## 🤨 Rationale

Refactor the Icon base class so new types of icons can extend the class
manually. Should be a non-breaking change for users as existing icon api
and paths don't change. Intention is to support custom icon types such
as multicolor icons (#327) and dynamic icons (#2813).

## 👩‍💻 Implementation

- Refactor the Icon base class to be abstract
- Make a new SVGIcon base class that implements the current
infrastructure for nimble icons

## 🧪 Testing

- Rely on existing tests

## ✅ Checklist

- [x] I have updated the project documentation to reflect my changes or
determined no changes are needed.

---------

Co-authored-by: rajsite <1588923+rajsite@users.noreply.github.com>
// <ok-dynamic-icon-rumham></ok-dynamic-icon-rumham>
```

#### Blazor Integration
Copy link
Member

@rajsite rajsite Jan 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Talking with @jattasNI put together a more end-to-end prototype that I think shows the pattern more clearly. Instead of having a declarative icon registration and renderer at the same time we will split the registration and rendering into separate parts.

Registration will be a programmatic API call from C#, i.e. a function like RegisterIconDynamicAsync. With the registration complete you have all the pieces you need to render in a table. The PoC PR #2827 shows what calling a registration from C# looks like and how it is used in the table. To take the PoC over the finish line we should move the JSInvoke calls from the example app and into the OkBlazor library. I added some comments about where they should go. In the example we have a helper JS Component with a static method for registering I think we should have a similar helper Blazor Component with a static method for registering. We'd then also want tests in the component, blazor, and storybook docs to finish up.

As a follow-on if you want to use the icons in spots outside the table we can add an addtional helper component to render the icon like <OkIconViewer Icon="ok-dynamic-icon-awesome"></OkIconViewer>. That's only if you also want to render dynamic icons in buttons, dropdowns, etc (the PoC branch linked does not demonstrate the Viewer but my understanding is for today you just need the Table support). The pattern suggested here is to align with the table mapping icon:

<NimbleMappingIcon Icon="ok-dynamic-icon-awesome"></NimbleMappingIcon>
<OkIconViewer Icon="ok-dynamic-icon-awesome"></OkIconViewer>

If the above sounds good and you want to take the PoC branch over the finish line with the above suggestions then we have enough detail captured here to avoid needing the complete HLD.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants